Skip to content

Conversation

@shua
Copy link

@shua shua commented Nov 25, 2025

This change is part of the allocator_api feature set #32838 (also see wg-allocators roadmap or libs-team ACP).
The previous attempts at adding an allocator parameter to string are at rust-lang/rust#101551, and rust-lang/rust#79500 (I think those authors should get much of the credit here, I am re-writing what they worked out in those threads).

workaround

There is a type inference ambiguity introduced when adding a generic parameter to a type which previously had none, even when that parameter has a default value (more details in rust-lang/rust#98931). I've done the same workaround as rust-lang/rust#101551, which is to make alloc::string::String a type alias to String<Global>, but I've arranged the modules a bit differently to make rebase/merges a bit easier.

This workaround unfortunately changes the type name of the String language item, and that would be user-facing in error or diagnostic output. I understand from this comment that this change is acceptable.

changes to existing API

Most of the methods on the original String have been implemented for the generic version instead. I don't foresee anything more moving from String<Global> to String<A>, as the remaining methods are all constructors which implicitly use the Global allocator.

There are three general types of changes:

  1. methods which don't allocate: here we just change impl Foo for String to impl<A: Allocator> Foo for String<A>
  2. converting to/from other allocator generic types like Vec<u8, A> and Box<str, A>: here we can use the existing allocator from those types.
  3. methods which clone from some other type with an allocator: here it's ambiguous whether the end result should be String<A>, String<Global>, or String<impl Allocator + Default>, etc; in general I try to leave these out of this change, but where some analogous change was made to Vec<T, A> I follow that.

new methods

Some methods have been added to String<A> which are not strictly necessary to land this change, but are considered helpful enough to users, and close enough to existing precedent in Vec<T, A>. Specifically, 4 new constructors (new_in, with_capacity_in, try_with_capacity_in, from_raw_parts_in), 1 destructor (into_raw_parts_with_alloc), and 1 getter (allocator). Technically, the updated from_utf8_unchecked should be sufficient for constructing, but we can add some safe constructors so users don't have to sully themselves.

not implemented

Variants of from_utf{8,16}* which internally allocate or use Cow have been punted on this PR, maybe a followup would make sense to either rewrite them, or add some *_in variant.

@rustbot
Copy link
Collaborator

rustbot commented Nov 25, 2025

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol, @tshepang

This PR modifies tests/ui/issues/. If this PR is adding new tests to tests/ui/issues/,
please refrain from doing so, and instead add it to more descriptive subdirectories.

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

@rustbot rustbot added A-rustc-dev-guide Area: rustc-dev-guide A-rustdoc-search Area: Rustdoc's search feature labels Nov 25, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 25, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Nov 25, 2025
@shua
Copy link
Author

shua commented Nov 25, 2025

maybe r? @Amanieu as the reviewer of the previous PR has more context

edit: whoops saw previous reviewer actually was Mark-Simulcrum, Amanieu was just last to leave a review. If it makes sense to swap it back, please do!

@rustbot rustbot assigned Amanieu and unassigned Mark-Simulacrum Nov 25, 2025
@cyrgani cyrgani added the A-allocators Area: Custom and system allocators label Nov 25, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added the T-release Relevant to the release subteam, which will review and decide on the PR/issue. label Nov 26, 2025
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Nov 26, 2025

☔ The latest upstream changes (presumably #147799) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot
Copy link
Collaborator

rustbot commented Nov 26, 2025

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rust-log-analyzer

This comment has been minimized.

shua added 2 commits November 26, 2025 23:08
This is a third attempt at implementing adding Allocator support to
the std lib's `String`.  Still stuck on the same issue with type
inference failing on the newly generic `String<A>`, but I opted to do
even less than the previous WIP work, and have added no new functions
(`String<A>` can be constructed via `Vec<u8, A>` or `Box<str, A>`),
and have moved only the struct definition to its own mod to make
rebasing a bit easier if/when main changes underneath me.
@rust-log-analyzer

This comment has been minimized.

shua added 6 commits November 27, 2025 09:02
This adds a diagnostic name `string_in_global` so that clippy can easily
check for the alloc::string::String type alias.
added some code in linkchecker to check the generic::String docs when
trying to resolve links to alloc::string::String type alias. There's
some lazy-loading that the browser does, but linkchecker doesn't, so
maybe some general-purpose solution would be better, but this seemed
better than a big list of exceptions.
Yes, you could just use `unsafe { from_utf8_unchecked }``, but people get
antsy about `unsafe`, so add some Vec ctor/dtor equivalents.
the links are changed in the original source, so not sure why the html
being checked in CI still has them, maybe it checks docs from `main` as
well, but if so, wouldn't `struct.String.html` still exist?

Truly a pickle, but I'll add these exceptions and a note.
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

...............................................F
======== tests/rustdoc-gui/item-decl-colors.goml ========

[ERROR] `tests/rustdoc-gui/item-decl-colors.goml` line 31
    from `tests/rustdoc-gui/item-decl-colors.goml` line 42: `.item-decl code .struct` not found: for command `assert-css: (".item-decl code .struct", {"color": |struct_color|}, ALL)`
[ERROR] `tests/rustdoc-gui/item-decl-colors.goml` line 31
    from `tests/rustdoc-gui/item-decl-colors.goml` line 56: `.item-decl code .struct` not found: for command `assert-css: (".item-decl code .struct", {"color": |struct_color|}, ALL)`
[ERROR] `tests/rustdoc-gui/item-decl-colors.goml` line 31
    from `tests/rustdoc-gui/item-decl-colors.goml` line 70: `.item-decl code .struct` not found: for command `assert-css: (".item-decl code .struct", {"color": |struct_color|}, ALL)`


<= doc-ui tests done: 47 succeeded, 97 failed, 0 filtered out

Error: ()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-allocators Area: Custom and system allocators A-rustc-dev-guide Area: rustc-dev-guide A-rustdoc-search Area: Rustdoc's search feature S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants